Node / meshtastic / Meshtastic-Android / files / .skills / code-review / SKILL.md

Displaying Rendered • View raw • Download

Perform comprehensive code reviews for T383838Meshtastic-Android, ensuring changes adhere to KMP architecture, Kotlin Multiplatform conventions, MAD standards, and CMP best practices.

When reviewing code, meticulously verify the following categories. Flag any deviations and propose the canonical project pattern as a fix.

• [ ] No Platform Bleed: Ensure absolutely no T383838java.* or T383838android.* imports exist in T383838commonMain source sets.

• [ ] KMP Native Alternatives: Verify the use of KMP alternatives for standard JVM libraries:

• T383838java.util.concurrent.locks.* -> T383838kotlinx.coroutines.sync.Mutex

• T383838java.util.concurrent.ConcurrentHashMap -> T383838atomicfu or Mutex-guarded T383838mutableMapOf()

• T383838java.io.* -> T383838Okio (T383838BufferedSource/T383838BufferedSink)

• T383838java.util.Locale -> Kotlin T383838uppercase()/T383838lowercase() (purged from T383838commonMain)

• [ ] Coroutine Safety: Use T383838safeCatching {} from T383838core:common instead of T383838runCatching {} in coroutine/suspend contexts. T383838runCatching silently swallows T383838CancellationException, breaking structured concurrency. Keep T383838runCatching only in cleanup/teardown code (abort, close, eviction). Use T383838kotlinx.coroutines.CancellationException (not T383838kotlin.coroutines.cancellation.CancellationException).

• [ ] Shared Helpers: If T383838androidMain and T383838jvmMain contain identical pure-Kotlin logic, mandate extracting it to a shared function in T383838commonMain.

• [ ] File Naming Conflicts: For T383838expect/T383838actual declarations, ensure files sharing the same package namespace have distinct names (e.g., keep T383838expect in T383838LogExporter.kt and shared helpers in T383838LogFormatter.kt) to avoid duplicate class errors on the JVM target.

• [ ] Interface & DI Over T383838expect/T383838actual: Check that T383838expect/T383838actual is reserved for small platform primitives. Interfaces + DI should be preferred for larger capabilities.

• [ ] Compose Multiplatform Resources: Ensure NO hardcoded strings. Must use T383838core:resources (e.g., T383838stringResource(Res.string.key) or asynchronous T383838getStringSuspend(Res.string.key) for ViewModels/Coroutines). NEVER use blocking T383838getString() in a coroutine.

• [ ] String Formatting: CMP only supports T383838%N$s and T383838%N$d. Flag any float formats (T383838%N$.1f) in Compose string resources; they must be pre-formatted using T383838NumberFormatter.format() from T383838core:common. Use T383838MetricFormatter for metric-specific displays (temperature, voltage, current, percent, humidity, pressure, SNR, RSSI).

• [ ] Centralized Dialogs & Alerts: Flag inline alert-rendering logic. Mandate the use of T383838AlertHost(alertManager) or T383838SharedDialogs from T383838core:ui/commonMain.

• [ ] Placeholders: Require T383838PlaceholderScreen(name) from T383838core:ui/commonMain for unimplemented desktop/JVM features. No inline placeholders in feature modules.

• [ ] Adaptive Layouts: Verify use of T383838currentWindowAdaptiveInfo(supportLargeAndXLargeWidth = true) to support desktop/tablet breakpoints (≥ 1200dp).

• [ ] Shared Navigation Graphs: Feature navigation graphs must be defined as extension functions on T383838EntryProviderScope<NavKey> in T383838commonMain (e.g., T383838fun EntryProviderScope<NavKey>.settingsGraph(...)). Flag any graphs defined in platform-specific source sets.

• [ ] Navigation Host: Ensure T383838MeshtasticNavDisplay (from T383838core:ui/commonMain) is used as the host instead of invoking T383838NavDisplay directly. Host modules should not configure T383838entryDecorators themselves.

• [ ] ViewModel Scoping: ViewModels obtained via T383838koinViewModel() must be inside T383838entry<T> blocks to correctly tie to the backstack lifetime.

• [ ] Annotation Usage: Ensure Koin is configured via annotations (T383838@Single, T383838@Factory, T383838@KoinViewModel).

• [ ] Root Assembly: Confirm that the root Koin DI graph is only assembled in host shells (T383838app and T383838desktop).

• [ ] Ktor Strictly: Check that Ktor is used for all HTTP networking. Flag and reject any usage of OkHttp.

• [ ] HTTP Configuration: Verify timeouts and base URLs use T383838HttpClientDefaults from T383838core:network. Never hardcode timeouts in feature modules. T383838DefaultRequest sets the base URL; feature API services use relative paths.

• [ ] Image Loading (Coil): Coil must use T383838coil-network-ktor3 in host modules. Feature modules should ONLY depend on T383838libs.coil (coil-compose) and never configure fetchers.

• [ ] Room KMP: Ensure T383838factory = { MeshtasticDatabaseConstructor.initialize() } is used in T383838Room.databaseBuilder. DAOs and Entities must reside in T383838commonMain.

• [ ] Room Patterns: Verify use of T383838@Upsert for insert-or-update logic. Check for T383838LIMIT 1 on single-row queries. Flag N+1 query patterns (loops calling single-row queries) — batch with chunked T383838WHERE IN instead.

• [ ] Bluetooth (BLE): All Bluetooth communication must be routed through T383838core:ble using Kable abstractions.

• [ ] JetBrains vs. AndroidX:

• In T383838commonMain: Must use T383838jetbrains-* aliases (e.g., T383838jetbrains-lifecycle-*, T383838jetbrains-navigation3-ui).

• In T383838androidMain: Can use T383838androidx-* or T383838jetbrains-* as appropriate, but do not mix them up in T383838commonMain.

• [ ] Compose Multiplatform: Ensure T383838compose-multiplatform-* aliases are used instead of plain T383838androidx.compose in all KMP modules.

• [ ] Test Placement: New Compose UI tests must go in T383838commonTest using T383838runComposeUiTest {} from T383838androidx.compose.ui.test.v2 (not the deprecated v1 T383838androidx.compose.ui.test package) + T383838kotlin.test.Test. Do not add T383838androidTest (instrumented) tests.

• [ ] Shared Test Utilities: Test fakes, doubles, and utilities should be placed in T383838core:testing.

• [ ] Libraries: Verify usage of T383838Turbine for Flow testing, T383838Kotest for property-based testing, and T383838Mokkery for mocking.

• [ ] Robolectric Configuration: Check that Compose UI tests running via Robolectric on JVM are pinned to T383838@Config(sdk = [34]) to prevent Java 21 / SDK 35 compatibility issues.

• [ ] New Dependencies: If a new reflection-heavy dependency is added (DI, serialization, JNI, ServiceLoader), verify keep rules exist in both T383838app/proguard-rules.pro (R8) and T383838desktop/proguard-rules.pro (ProGuard). The two files must stay aligned.

• [ ] Release Smoke-Test: For dependency or ProGuard rule changes, verify T383838assembleRelease and T383838./gradlew :desktop:runRelease succeed.

1. Be Specific & Constructive: Provide exact file references and code snippets illustrating the required project pattern.

2. Reference the Docs: Cite T383838AGENTS.md and project architecture playbooks to justify change requests (e.g., "Per AGENTS.md, T383838java.io.* cannot be used in T383838commonMain; please migrate to Okio").

3. Enforce Build Health: Remind authors to run T383838./gradlew test allTests locally to verify changes, especially since KMP T383838test tasks are ambiguous.

4. Praise Good Patterns: Acknowledge correct usage of complex architecture requirements, like proper Navigation 3 scene transitions or elegant T383838commonMain helper extractions.